Replace requests with request and coding style improvements#33
Replace requests with request and coding style improvements#33palasso wants to merge 15 commits intoGamah:masterfrom
Conversation
palasso
commented
Dec 25, 2015
- Replace requests calls with request and remove requests dependency
- Remove unused variables, imports
- Coding style improvements, fixes examples in Apply consistent coding style #32
- Make it more readable
Coding style improvement: Replace import urllib.request with from urllib import request and do appropriate changes in code
Coding Style Improvement: Replace ( ) with () and function () with function() to make code style consistent
|
Firstly, please title these better, explain your changes at a glance. Secondly, I'd like to remove the 3rd party reliance on the requests module as a whole, you'll win me over much more quickly if you re-implement the commands which rely on requests with urllib.request instead for this PR. As I'm likely commit my own changes otherwise before committing to master. |
|
How are we gonna parse URL titles and wisdoms of Chopra without requests? Also why not just keep it, it's comfy and is meant to be a better request. |
|
I kept this as 'palasso PR' because I'll add more to the PR as time goes. The other PR would have its name changed every once in a while resulting in a big title. I will have in 1st comment a bulleted list on all the fixes this PR does with their corresponding issues from the bugtracker and the commits are also descriptive. |
|
There's no need to use both the external requests module and the request class from the built in urllib module. I'm pretty sure it's possible to get rid of the requests module entirely. |
|
Ok but why not get rid of request? requests has more functionality, we'd have to recreate it. I don't see the benefit of it. Better re-use good libraries I say :P |
|
There isn't anything fundamentally that the bot needs to do that requires On Fri, Dec 25, 2015, 3:34 PM palasso notifications@github.com wrote:
|
|
i will likely remove the user of requests in the near future. Before I bother with that I will vet these changes and merge (assuming you make no further commits before that. Something you may consider is creating your own branches for specific changes so that different topics can become different pull requests, as opposed to generic pull requests with multiple different topics per commit. So long as you aren't mucking about with the same lines too often, it likely wouldn't result in too many merge conflics (especially if I merge in chronological order) and will result in a more easy to follow history of merges. |
|
I also believe creating separate PRs for separate issues :) Only reason started it as generic PR was because of previous recommendations on previous PR and on CONTRIBUTING file saying: "Please do not submit PR's for issues for special case issues." btw this PR is tested and ready to be merged. Don't plan to add anything else regarding coding style improvements in the near future. |
Update to avoid conflict
Remove requests dependency and replace requests calls with request
|
There would be a conflict so decided to implement here the removal of requests as well. This branch is tested and ready to be merged. |
|
So this has rotten for 3 months here. Some commits happened in master afterwards so now it has conflicts. So what's going to happen with that? btw it has a conflict due to last commit in master. If it's considered to be merged the simplest way to do this would be to undo latest commit in master, merge this and then add the removed commit in master. |